feat: two-safe rule - #12
Conversation
Assisted-by: Claude:claude-sonnet-4-6
Assisted-by: Claude:claude-sonnet-4-6
Assisted-by: Claude:claude-sonnet-4-6
Assisted-by: Claude:claude-sonnet-4-6
Assisted-by: Claude:claude-sonnet-4-6
I've thought of a better way to do this. |
Assisted-by: Claude:claude-sonnet-4-6
Assisted-by: Claude:claude-sonnet-4-6
Assisted-by: Claude:claude-sonnet-4-6
Assisted-by: Claude:claude-sonnet-4-6
…ions Assisted-by: Claude:claude-sonnet-4-6
Assisted-by: Claude:claude-sonnet-4-6
Assisted-by: Claude:claude-sonnet-4-6
Assisted-by: Claude:claude-sonnet-4-6
| OwnerSet allOwners = OwnersLibrary.getAllOwners(); | ||
|
|
||
| // modify | ||
| if (loaded.approvals & allOwners == allOwners) { |
There was a problem hiding this comment.
| if (loaded.approvals & allOwners == allOwners) { | |
| if ((loaded.approvals & allOwners) == allOwners) { |
does the linter have a problem with this? easy to misread this (and the one on line 56, and I guess 49 has the same shape); it's one char away from && so in an if it just looks wrong.
There was a problem hiding this comment.
does the linter have a problem with this?
No.
it's one char away from && so in an if it just looks wrong
It's the same bitwise arithmetic syntax as other languages. I would normally only add extra parentheses to avoid readers having to lookup the operator precedence rules, which can differ language-to-language. == is not such a situation.
Another reason this & won't be confused with && is that then the first evaluation would be allOwners == allOwners, always true.
Besides bitwise operations, languages Python and C++ commonly use the bitwise operators & and | for bitset operations like this one.
There was a problem hiding this comment.
yeah, I get all of this, all very true, but I think in cases like this where there's a possibility of misreading or someone not having robotic recall of precedence rules in their brains its always good to be as clear as possible, parentheses don't hurt, they don't change the code, but they do make code incredibly clear where precedence rules or syntax isn't doing the work of adding clarity; an extremely cheap addition to reduce cognitive overhead
From the Owners library raises an important point, but also I don't believe this is either actioned in the code or actionable in the current form. Mutating the |
No. The bitmasking handles this. I think if you spell out the exact way that it could get us into trouble, you would realize how ridiculous it is. First, it's not true that the tasks would be unable to complete. Second, the real danger is that a stale task from 159 owners ago might be passively approved by the next owner, and this isn't prevented at all by your proposed mitigation. However it is resolved by my mitigation. If a task is really sitting stale through 159 owners, it should have been vetoed. |
|
Follow-up on visibility: #14 re removals:
|
Assisted-by: Claude:claude-sonnet-4-6
github-project-automation
Bot
moved this from 📌 Triage
to ✔️ Approved by reviewer
in FOC
3 participants
Closes #2
UnanimousGovernanceThis implements the two-safe rule as it will be used for discretionary actions in the smart contracts:
Some actions require consensus of the owners, and are annotated with the
unanimousmodifier. They may also have aholdperiod, measured in epochs. If an action has unanimous consent it will either take immediate effect or, after the mandatory holding period, anyone can trigger its completion. During a holding period, any owner can cancel the operation.Events:
OwnersThe multiple owners are tracked in
Owners. Their approvals are tracked with a bitset in order to minimize the PendingTask storage footprint (1 slot per pending action). There is also a mapping to allowO(1)isOwner checking, and to track representative bits for the bitset.Events: